Repository navigation
fix(types): accept an explicit undefined wherever a consumer writes an optional member - #418
YevheniiKotyrlo wants to merge 1 commit into
Conversation
|
| errors in the file | |
|---|---|
main (df32ce30) |
three TS2769: No overload matches this call, one per element |
| this PR | none |
Importing the package built with yarn build, with /// <reference types="react-native-css/types" />, the same elements, the mapping snippet from the body, and the options of styled(), compile, registerCSS and withReactNativeCSS each written undefined give 8 errors on main and none on this PR; with skipLibCheck off, the package's built declarations report nothing on either.
A mapping entry left undefined — main against this PR
The snippet from the body, rendered through the repo's jest with a window.dispatchEvent stub, so React's report of the error it recovered from reaches console.error, against the sources and against the built dist/commonjs alike:
| errors | accessibilityLabel |
style |
|
|---|---|---|---|
main (df32ce30) |
There was an error during concurrent rendering but React was able to recover by instead synchronously rendering the entire root., caused by Cannot read properties of undefined (reading 'split') |
#00f |
{} |
| this PR | none | #00f |
{ backgroundColor: "#f00" } |
Windows 11 Pro 10.0.26300, Node 26.12.1, TypeScript 7.0.2 and 6.0.3, which give the same results.
6c6da94 to
d10ad90
Compare
8c8919f to
49ff1c9
Compare
e283081 to
157684a
Compare
sammoore
left a comment
There was a problem hiding this comment.
At first glance, would love to merge the JSX types error caused by exactOptionalPropertyTypes, but there's a bunch of seemingly unrelated / heavy-handed changes in here:
NativeStyleMappingvalidation (we can just filterundefinedvalues to avoid the resulting runtime error for now), the newisPath(also changes runtime semantics for something seemingly unrelated), etc- the parentheses around the logger fn vs.
Debugger(great catch! would love to merge this) skills/updates
Generally speaking, the minimal fix here for the issue as outlined in the description seems to just be updating the types to include | undefined and then filtering the undefined.
Let me know what your thoughts @YevheniiKotyrlo
|
@sammoore yes, you are right, this PR is a bit poor taste from my side with some indeed unrelated changes in the docs and etc. I believe this changes slipped in from my patch regeneration tool that I had to write to constantly regenerate patches from my PRs on top of current version I was using. I will code review it and see how I can improve it. Thanks! |
…n optional member
Under exactOptionalPropertyTypes a bare `?: T` rejects the explicit
undefined React Native's own optional props accept, so
`className={condition ? "p-4" : undefined}` did not compile. Every
optional member a consumer writes now takes `| undefined`: the props
types.d.ts adds, styled()'s props, options and mapping, the options of
the compiler, Metro, the jest helper and useNativeCss, and the Effect an
observable's get() takes. CompilerOptions.logger's function type is
parenthesised, so Debugger is no longer read as its return type.
A mapping entry left undefined now type-checks, and mappingToConfig
drops it rather than passing it on to the native style mapping, which
called `.split(".")` on it at render.
optional-members.test.ts walks, with the type checker and the flag on,
what a consumer writes to each entry point package.json exports, and
fails on any optional member that rejects undefined.
157684a to
6d12eb2
Compare
|
@sammoore the branch is now one commit on current
|
Problem
React Native declares its own optional props as
?: T | undefined, while the props this package adds are bare?: string. UnderexactOptionalPropertyTypesthe two differ, so this fails to compile:styled()has the same bare members: theclassNameits unmapped overload adds,passThrough, and a mapping'snativeStyleMappingand its entries; so do the options a consumer writes for the compiler, Metro, the jest helper anduseNativeCss.A
nativeStyleMappingentry leftundefinedis also a runtime error:mappingToConfigkeeps it, and the native style mapping callspath.split(".")on it:Solution
| undefined: the propstypes.d.tsadds; the propsstyled()adds,passThrough,nativeStyleMappingand its entries inruntime.types.ts;CompilerOptions, Metro's options, the jest helper'sdebuganduseNativeCss'sConfig; and theEffectthatget()takes on the observablesStyleCollectionexposes. Without the flag the types are unchanged.CompilerOptions.loggeris((message: string) => void) | Debugger | undefined, where(message: string) => void | DebuggermadeDebuggerthe function's return type.mappingToConfigdrops anativeStyleMappingentry leftundefined.Tests
src/__tests__/native/styled.test.ios.tsx: a mapping entry set toundefinedmaps nothing, and the entry beside it still maps. Onmainits render throws, andyarn typecheckrejects itsundefinedentry.src/__tests__/types/optional-members.test.tsbuilds a program withexactOptionalPropertyTypeson and walks what a consumer writes: the propstypes.d.tsadds,runtime.types.ts, and what each entry point ofpackage.json'sexportstakes. It fails on any optional member that rejectsundefined, so a member added later is held to it too; onmainit names the 42 places this PR widens.Verification
yarn lint·yarn typecheck·yarn test1418 passed ·yarn buildandnode --test node/__tests__/*.mjs·yarn example expo export --platform web·commitlint: all green on this head on Ubuntu, with Node 26.12.1 (.nvmrcpins 22.18.0), and the iOS dev build on macOS 27.2 with Xcode 27.2, whereci.ymlpins 26.6.The
classNameexample above and the mapping snippet, each onmainand on this branch, against the sources and against the built package, are in this comment.In an Expo app,
expo/typesdeclaresclassNameonViewProps,TextPropsand other React Native props without| undefined, andexpo-env.d.tsloads it before this package's declarations, so there theclassNameexample keeps failing until Expo's declarations take| undefinedtoo, which expo/expo#43508 tracks.Merge order
dirtoViewPropsandTextPropsintypes.d.ts; whichever lands second gives it| undefined, whichoptional-members.test.tschecks.nativeStyleMappingdeclaration insrc/runtime.types.ts, and fix(native): give a target:false config its own scratch space, and never map onto the cached props #435 makesConfig's members read-only insrc/native/react/useNativeCss.ts; whichever lands second keeps both changes,| undefinedincluded.